Fix pluralized lifecycle status operationIds (closes #475)#497
Conversation
et_lc.first ('apps'/'components') was used for operationId construction,
producing pluralized names like getAppsStatus and getComponentsStatus.
Every other route uses the singular form (getAppDataItem, listComponentFaults).
Changed to et_lc.second ('app'/'component') for consistency.
There was a problem hiding this comment.
Pull request overview
Fixes issue #475 by changing OpenAPI operationId generation for lifecycle status routes to use the singular entity name (app / component) instead of the plural collection name (apps / components), aligning these IDs with the naming convention used across the rest of the API and improving generated client method names.
Changes:
- Update lifecycle status route registration to build
operationIds fromet_lc.second(singular) rather thanet_lc.first(plural).
| // e.g. "App" / "Component" for operation ID construction | ||
| const std::string entity_cap = capitalize(std::string(et_lc.second)); |
bburda
left a comment
bburda
left a comment
There was a problem hiding this comment.
@YueBit the fix is correct.
Suggestion: add a regression guard on the generated OpenAPI spec, so a revert to the plural names would fail a test. Nothing asserts the operationId values today. The callability test reads operationId only as a label, and the uniqueness check in the health test still passes with the plural form, so both miss this.
The status routes only appear in the root spec at GET /docs (the entity-scoped /apps/docs sub-spec does not list them). The existing test_root_docs_returns_openapi_spec in src/ros2_medkit_integration_tests/test/features/test_docs_endpoint.test.py already fetches the root spec and keeps paths in scope, so a few asserts fit there:
# Lifecycle status operationIds use the singular entity name
# (getAppStatus), not the plural collection name (getAppsStatus).
self.assertEqual(
paths['/apps/{app_id}/status']['get']['operationId'], 'getAppStatus')
self.assertEqual(
paths['/components/{component_id}/status']['get']['operationId'],
'getComponentStatus')
self.assertEqual(
paths['/apps/{app_id}/status/restart']['put']['operationId'],
'putAppStatusRestart')Run it with:
./scripts/test.sh test_docs_endpoint
Assert that root OpenAPI spec uses singular entity names in lifecycle status operationIds (getAppStatus, getComponentStatus), not the plural collection names (getAppsStatus, getComponentsStatus). This prevents regression of the fix in selfpatch#475.
|
Done, thanks for the suggestion! I've added the regression assertions to The test passed successfully:
|
Pull Request
Summary
Fix #475 by using the singular entity name when constructing lifecycle status operationIds.
Previously the code used et_lc.first ("apps"/"components"), producing names such as getAppsStatus and putAppsStatusRestart.
This changes the operationId generation to use et_lc.second ("app"/"component"), matching the naming convention used throughout the rest of the API.
Issue
Link the related issue (required):
Type
Testing
Builds under ROS 2 Humble.
No runtime behavioral change. Endpoints, handlers, and responses are unchanged; only the generated OpenAPI operationId values are affected.
Checklist